Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

swipe tabs #2370

Merged
merged 46 commits into from
Feb 9, 2024
Merged

swipe tabs #2370

merged 46 commits into from
Feb 9, 2024

Conversation

brindy
Copy link
Contributor

@brindy brindy commented Jan 23, 2024

Task/Issue URL: https://app.asana.com/0/72649045549333/1205842534063579/f
Tech Design URL:
CC:

Description:
Add swipe tabs functionality.

Steps to test this PR:

Internal User:

  1. This feature is behind the internal user flag. When this is off then swipe tabs should not work on any device.

Phone:

  1. When on Home Screen with no tabs: no scrolling
  2. When on a single tab on a web page: swiping left will launch a new tab. Note that if there are favourites they will not be shown until a 'preview' of the tab has been captured, so will show Dax the first time you swipe to a new tab
  3. Swiping between pages will show a preview if one has been captured. Web pages not loaded yet will start to load (same as if opening from the tab switcher).
  4. Switching orientation will scale the preview, but won't align it with the scroll position on the page in the other orientation.
  5. Check the fire button
  6. Check opening a new tab from long press the tab button
  7. Check opening a new tab from the tab switcher

iPad:

  1. Swipe tabs will only show when in the "iPhone view" - e.g. the small split screen view.
  2. When in iPad view no swiping should work and the tabs bar should be visible

Themes:

  1. Check light and dark mode.

OS Version:

  1. Should work for all supported OS. Ideally check 14, 15, 16 and 17 (I've tested 16 and 17 as don't have device/simulator for 14/15 - ping me if it's a problem and I'll get them).

Copy link

github-actions bot commented Jan 29, 2024

Warnings
⚠️ PR has more than 500 lines of code changing. Consider splitting into smaller PRs if possible.

Generated by 🚫 dangerJS against d117156

@brindy brindy marked this pull request as ready for review February 5, 2024 17:21
@samsymons
Copy link
Contributor

@brindy One issue I'm running into today is a pretty persistent flickering when changing tabs:

RPReplay_Final1707260399.2.MP4

@samsymons samsymons self-requested a review February 9, 2024 04:51
Copy link
Contributor

@samsymons samsymons left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Besides the flickering issue I mentioned, this is all behaving well. I found a nasty memory leak, but was able to reproduce it on main so I'll raise it elsewhere - it's not introduced by this PR.

Code changes seem sensible and I've not been able to break anything on any device with various iOS versions. Let's merge it and give it a go internally. Nice work!

@brindy brindy merged commit 0513a1d into main Feb 9, 2024
10 checks passed
@brindy brindy deleted the brindy/swipe-tabs branch February 9, 2024 09:30
samsymons added a commit that referenced this pull request Feb 12, 2024
* main:
  Bump submodules/privacy-reference-tests from `a3acc21` to `6b7ad1e` (#2460)
  Fix HomeViewController retain cycle (#2462)
  Fix hotfixing process (#2443)
  Daniel/subscriptions/8.itp (#2427)
  Add strict checking for Asana link in PR description (#2463)
  PR test (#2464)
  swipe tabs (#2370)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants